feat: add transplanted 1D quadrature maps, docs, tests, and QBX example#159
feat: add transplanted 1D quadrature maps, docs, tests, and QBX example#159xywei wants to merge 6 commits intoinducer:mainfrom
Conversation
|
Thanks! Could you look at those linter failures? The biggest one is probably adding types. |
|
(I can help if there's an issue.) |
|
cc @ShawnL00 |
There was a problem hiding this comment.
Pull request overview
Adds transplanted 1D quadrature rules/maps (Hale–Trefethen and Kosloff–Tal-Ezer) to modepy, with accompanying docs, tests, and a QBX comparison example.
Changes:
- Introduces
modepy.quadrature.transplantedwith transplant maps, a dispatcher, and transplanted quadrature classes. - Exports transplanted quadrature classes through package init modules (
modepy.quadratureand top-levelmodepy). - Adds docs + tests for map dispatch/validation and includes a 2D QBX comparison example script.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
modepy/quadrature/transplanted.py |
New transplanted-map implementations and transplanted quadrature classes. |
modepy/quadrature/__init__.py |
Re-exports transplanted quadrature classes from the quadrature package. |
modepy/__init__.py |
Exposes transplanted quadrature classes at the top-level modepy API. |
modepy/test/test_quadrature.py |
Adds unit tests for transplanted quadrature mapping/validation behavior. |
examples/plot-qbx-transplanted-vs-gauss.py |
New QBX comparison example for Gauss vs transplanted rules. |
doc/quadrature.rst |
Adds documentation section describing transplanted quadrature usage and references. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| OUT = "/tmp/qbx-transplanted-vs-gauss-2d.png" | ||
|
|
There was a problem hiding this comment.
The example writes output to a hard-coded /tmp/... path, which will fail on non-POSIX platforms (e.g. Windows) and can be surprising for users.
Consider using tempfile.gettempdir()/pathlib.Path, saving relative to the current working directory, or making the output path a CLI argument.
| mapped_nodes, jacobian = map_trefethen_transplant( | ||
| nodes_1d, | ||
| map_name=map_name, | ||
| strip_rho=strip_rho, | ||
| kte_rho=kte_rho, | ||
| kte_alpha=kte_alpha, | ||
| ) | ||
| mapped_weights = quadrature.weights * jacobian | ||
|
|
||
| if force_dim_axis: | ||
| mapped_nodes = np.reshape(mapped_nodes, (1, mapped_nodes.shape[0])) | ||
|
|
||
| super().__init__(mapped_nodes, mapped_weights) | ||
|
|
There was a problem hiding this comment.
Transplanted1DQuadrature drops the base rule's exact_to information even when map_name="identity" (and similarly for identity-equivalent maps like sausage_d1). That makes q.exact_to raise AttributeError despite the transplanted rule being identical to the base rule in that case.
Consider preserving exact_to when the map is exactly the identity, while leaving it unset for non-identity maps where polynomial exactness in x is generally not preserved.
alexfikl
left a comment
There was a problem hiding this comment.
Very nifty! 😁
I took a quick look and left some nitpicks and questions.
| :members: | ||
| :show-inheritance: | ||
|
|
||
| Transplanted quadrature in one dimension |
There was a problem hiding this comment.
I don't feel particularly strongly about this, but I haven't heard the term "transplanted" used very often, so it sounds a bit unintuitive to me. Maybe call this mapped or conformal mapped? Maybe just transformed? Although we already have a Transformed1DQuadrature that just does linear transformations.
For context, I'm mostly familiar with stuff like this from Kress' Numerical Analysis, e.g. Chapter 9.6 and some of the cited references, so it might just be a different tradition.
(Same for other places where the term is used, depending on what's decided.)
|
|
||
| * N. Hale and L. N. Trefethen, *New Quadrature Formulas from Conformal | ||
| Maps*, ``SIAM Journal on Numerical Analysis`` 46(2), 930-948 (2008), | ||
| doi:10.1137/07068607X. |
There was a problem hiding this comment.
| doi:10.1137/07068607X. | |
| `doi:10.1137/07068607X <https://doi.org/10.1137/07068607X>`__. |
? Same in other places. Also as far as formatting goes, it looks a bit weird to put the journal in code mode (?).
| Maps*, ``SIAM Journal on Numerical Analysis`` 46(2), 930-948 (2008), | ||
| doi:10.1137/07068607X. | ||
| * D. Kosloff and H. Tal-Ezer, *A Modified Chebyshev Pseudospectral Method | ||
| with an O(N^{-1}) Time Step Restriction*, |
There was a problem hiding this comment.
| with an O(N^{-1}) Time Step Restriction*, | |
| with an :math:`O(N^{-1})` Time Step Restriction*, |
? I haven't actually checked how this renders.
| @@ -0,0 +1,292 @@ | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
This example is pretty awesome, but I'm wondering if it's too big for modepy? Maybe better to just put it in pytential once this gets in?
For modepy, I would appreciate just a little example from the paper, e.g. Figure 7.1 or similar (doesn't need to be all those functions).
| Transplanted quadrature applies a smooth map :math:`x=g(s)` to an existing | ||
| one-dimensional rule on :math:`[-1,1]`. | ||
| Given base nodes/weights :math:`(s_i, w_i)`, the transplanted rule is |
There was a problem hiding this comment.
| Given base nodes/weights :math:`(s_i, w_i)`, the transplanted rule is | |
| Given base nodes/weights :math:`(s_i, w_i')`, the transplanted rule is |
? (add a little prime to w or use \omega? to avoid confusion)
| sausage_degree = _sausage_degree_from_map_name(map_name) | ||
| if sausage_degree is not None: | ||
| return map_sausage(s, sausage_degree) |
There was a problem hiding this comment.
Hm, would it be clearer to just add a sausage_degree parameter and let the map name be sausage? Similar to how the other maps have their kte_rho etc parameters in here.
It avoids the need to parse the int from the name at least.
| if map_name == "strip": | ||
| return map_strip(s, rho=strip_rho) |
There was a problem hiding this comment.
I don't imagine we'll be calling these in a hot loop, so the order probably doesn't matter, but would it make sense to put the strip first? My understanding is that it's the latest and the greatest, so likely to be the most used in practice.
| Reference: | ||
| N. Hale and L. N. Trefethen, "New Quadrature Formulas from | ||
| Conformal Maps," SIAM Journal on Numerical Analysis 46(2), | ||
| 930-948 (2008), | ||
| doi:10.1137/07068607X. | ||
| D. Kosloff and H. Tal-Ezer, "A Modified Chebyshev Pseudospectral | ||
| Method with an O(N^{-1}) Time Step Restriction," Journal of | ||
| Computational Physics 104(2), 457-469 (1993), | ||
| doi:10.1006/jcph.1993.1044. |
There was a problem hiding this comment.
Same, just put these references in a central place.
| ) | ||
|
|
||
|
|
||
| class Transplanted1DQuadrature(Quadrature): |
There was a problem hiding this comment.
Does this need to be a separate quadrature class? I would expect map_trefethen_transplant (or a small wrapper) to just take a Quadrature and return a Quadrature with custom nodes + weights?
I don't feel particularly strongly about that though, since it's mostly all the same. I guess if we have Transformed1DQuadrature, this makes sense too.
| from modepy.quadrature.transplanted import ( | ||
| Transplanted1DQuadrature as Transplanted1DQuadrature, | ||
| TransplantedLegendreGaussQuadrature as TransplantedLegendreGaussQuadrature, | ||
| ) |
There was a problem hiding this comment.
Should be fine to let these just be imported from quadrature.transplanted? The other quadrature rules also just have a shortcut in modepy.__init__.
Summary
Add
modepy.quadrature.transplantedwith transplanted 1D quadrature support:identity, genericsausage_d{odd},strip, andkte/kosloff_tal_ezermap_trefethen_transplantTransplanted1DQuadratureandTransplantedLegendreGaussQuadratureExport transplanted quadrature classes from package init modules.
Expand quadrature docs with dedicated transplanted sections:
strip_rho,kte_rho,kte_alpha)Add a 2D QBX comparison example:
examples/plot-qbx-transplanted-vs-gauss.pyAdd transplanted quadrature tests:
Validation
python examples/plot-qbx-transplanted-vs-gauss.pypytest modepy/test/test_quadrature.py -k transplanted